Skip to content

transport+visor: retire on-disk CSV transport-log store; serve history from stats bbolt#2427

Merged
0pcom merged 1 commit intoskycoin:developfrom
0pcom:fix/drop-csv-transport-logs
May 4, 2026
Merged

transport+visor: retire on-disk CSV transport-log store; serve history from stats bbolt#2427
0pcom merged 1 commit intoskycoin:developfrom
0pcom:fix/drop-csv-transport-logs

Conversation

@0pcom
Copy link
Copy Markdown
Collaborator

@0pcom 0pcom commented May 4, 2026

Summary

Retire the per-transport CSV log store. On a public visor it was the dominant alloc-rate symbol after #2420 landed — pprof showed transport.fileTransportLogStore.writeToCSV at ~78% of cumulative bytes allocated, going through gocsv reflection-based marshalling for every packet on every ManagedTransport. The CSV files were also strictly narrower than data we already publish: the pkg/visor/stats bbolt store carries the same bandwidth + latency snapshots, with daily rollups already feeding the CXO publisher subscribed by the TPD aggregator.

This PR removes the file-backed store entirely and re-points GetTransportLogs() (the only consumer of the CSVs — used by skywire-cli tp -b N for bandwidth history) at the bbolt stats store.

Changes

Removed code

  • pkg/transport/log.go — drop fileTransportLogStore (writeToCSV / readFromCSV / recoverCSV / repairCSVFile / cleanLogs). Drop the parallel fileLatencyLogStore along with its interface LatencyLogStore, inMemoryLatencyLogStore, LatencyCsvEntry — the manager held a LatencyLogStore field but nothing in the codebase ever called Record on it, pure dead weight. CsvEntry and the gocsv/encoding/csv/bufio imports go with them.
  • pkg/transport/manager.go — drop the unused LatencyLogStore field on ManagerConfig.
  • pkg/transport/log_test.go — drop TestFileTransportLogStore.
  • pkg/visor/init_transport.go — collapse the file/memory branch to always-memory; drop the latency-store init. LogStore.Type=\"file\" in existing visor configs is accepted but logged as deprecated.
  • pkg/visor/logserver/api.go — drop the /transport_logs/:file HTTP route and its landing-page listing. The same data is at /stats/transports/history (live snapshot at /stats/transports). Unused tpLogPath parameter on logserver.New stays in the signature for call-site compat (init_dmsg.go passes it).
  • pkg/skyenv/skyenv.go — drop the unused LatencyLogStore directory constant. TpLogStore stays since cmd/skywire-cli/commands/config/gen.go still emits it as the default LogStore.Location for backward-compatible configs.

Re-pointed read path

  • pkg/visor/api_transport.goGetTransportLogs(days int) now walks stats.Store.AllTransportRecords() and emits one entry per (transport, day) within the requested window. The CSV store's RecvBytes/SentBytes was the within-day delta (each daily file was zero-anchored at day rollover via the explicit Reset() in writeToCSV), which matches DailyRollup's delta semantics exactly. skywire-cli tp -b N aggregation is unchanged. Drop the local csvEntry parser and readTransportLogFile.

Diff

 pkg/skyenv/skyenv.go        |   9 +-
 pkg/transport/log.go        | 537 +-------------------------------------
 pkg/transport/log_test.go   |  24 --
 pkg/transport/manager.go    |   1 -
 pkg/visor/api_transport.go  |  96 +++----
 pkg/visor/init_transport.go |  37 +--
 pkg/visor/logserver/api.go  |  28 +--
 7 files changed, 68 insertions(+), 664 deletions(-)

Behavioural notes

  • Live cumulative byte counters (skywire-cli tp ls, hypervisor UI per-transport bytes): same in-memory semantics as today's memory-mode users — "bytes since transport became live in this visor session." File-mode users used to keep counters across visor restarts via the daily CSV; that no longer happens. Daily totals via tp -b N are unaffected because bbolt is the source of truth.
  • /transport_logs/<date>.csv HTTP endpoint is removed. Functionally replaced by /stats/transports/history?since=&until=&id= already wired into the same logserver.

Test plan

  • go test ./pkg/transport/... (transport, addrresolver, handshake, tpdclient — all green)
  • go test ./pkg/visor/... (visor, dmsgtracker, logserver, stats, usermanager, visorconfig — all green)
  • go build ./... — whole-repo build clean
  • skywire -b runs on the resulting binary
  • Verify on a live public visor: rerun pprof and confirm writeToCSV is gone from the alloc profile; verify skywire-cli tp -b 7 still returns sensible per-day bandwidth aggregated from the bbolt store

…y from stats bbolt

The per-transport CSV log store wrote a row to a daily file on every
packet via gocsv reflection-based marshalling. On a public visor that
turned into the dominant alloc-rate symbol — pprof showed
transport.fileTransportLogStore.writeToCSV at ~78% cumulative bytes
allocated, dwarfing every other hot path once the CXO publisher's
own allocation churn was fixed (skycoin#2420). The CSV files were also
narrower than the bbolt-backed stats store that already exists
(no latency, less precise timestamps), and the stats store was
already plumbed into the CXO publisher feeding TPD's aggregator.

Drop the file-backed log stores entirely:

- pkg/transport/log.go — remove fileTransportLogStore and the
  parallel fileLatencyLogStore (the latter was wired into
  ManagerConfig but never read by anything — pure dead weight).
  CsvEntry / LatencyCsvEntry gocsv structs go with them. The
  in-memory LogStore stays — ManagedTransport uses it to preserve
  cumulative byte counters across same-session reconnects of the
  same transport ID.

- pkg/transport/manager.go — drop the unused LatencyLogStore
  field on ManagerConfig.

- pkg/visor/init_transport.go — collapse the file/memory branch to
  always-memory; drop the latency-store init. LogStore.Type=file
  in existing visor configs is accepted but logged as deprecated.

- pkg/visor/api_transport.go — re-implement GetTransportLogs to
  walk stats.Store.AllTransportRecords() and emit one entry per
  (transport, day) within the requested window. The CSV path's
  RecvBytes/SentBytes was the within-day delta (CSVs were
  zero-anchored at each day rollover), which matches DailyRollup
  semantics exactly — `skywire-cli tp -b N` aggregation is
  unchanged. Drop the local csvEntry parser and readTransportLogFile.

- pkg/visor/logserver/api.go — drop the /transport_logs/:file
  HTTP route and its landing-page listing. Same data is served by
  /stats/transports/history (live snapshot at /stats/transports).
  The unused tpLogPath parameter on logserver.New stays in the
  signature for call-site compat (init_dmsg.go passes it).

- pkg/skyenv/skyenv.go — drop the unused LatencyLogStore directory
  constant; leave TpLogStore in place since gen.go still emits it
  as the LogStore.Location default for backward-compatible configs
  (the runtime ignores the path — nothing is written there).

Cumulative bytes counters on per-transport status (skywire-cli tp ls,
hypervisor UI) still reflect "bytes since transport became live in
this visor session" — same in-memory semantics that file-mode users
also got within a single session. Across visor restarts everyone
starts fresh, which is what memory-mode users were already
experiencing.

Tests: package tests pass (transport, visor, visor/logserver,
visor/stats, visor/visorconfig). The only remaining file-mode test
(TestFileTransportLogStore) is removed along with the
implementation.
@0pcom 0pcom merged commit ebde67f into skycoin:develop May 4, 2026
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant